src/routes/(console)/project-[region]-[project]/sites/site-[site]/+page.ts (1)

103-110: Avoid swallowing all fetch errors silently.

At Line 108 and Line 129, returning null for every exception hides operational failures. Consider at least logging unexpected errors so production issues remain debuggable.

Suggested pattern

 async function loadDeployment({
@@
 }): Promise<Models.Deployment | null> {
     try {
         return await sdk.forProject(region, project).sites.getDeployment({
             siteId,
             deploymentId
         });
     } catch (error) {
+        console.error('Failed to load site deployment', { siteId, deploymentId, error });
         return null;
     }
 }
@@
 }): Promise<Models.ProviderRepository | null> {
     try {
         return await sdk.forProject(region, project).vcs.getRepository({
             installationId,
             providerRepositoryId
         });
     } catch (error) {
+        console.error('Failed to load site repository', {
+            installationId,
+            providerRepositoryId,
+            error
+        });
         return null;
     }
 }

Also applies to: 124-131

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/routes/`(console)/project-[region]-[project]/sites/site-[site]/+page.ts
around lines 103 - 110, The catch block that wraps
sdk.forProject(...).sites.getDeployment currently swallows all errors and
returns null; change it to detect and handle expected "not found" responses
(e.g., check error.status === 404 or SDK-specific NotFound error) returning null
only in that case, but for all other exceptions log the error with context
(include region, project, siteId, deploymentId) using the project's logger (or
console.error if no logger available) and rethrow or return a meaningful error;
update the catch around sdk.forProject(...).sites.getDeployment (and the similar
block calling the same method) accordingly so operational failures are not
silently hidden.

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

src/lib/components/git/deploymentSource.svelte (1)

29-41: Optional: Add explicit type to authorized for consistency.

TypeScript strict null checks are currently disabled in this project, so the current declaration works without issues. However, for consistency with similar state declarations elsewhere (e.g., multiSelectTable.svelte), consider explicitly typing the state:

♻️ Proposed fix

-    let authorized = $state(null);
+    let authorized = $state<boolean | null>(null);

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/lib/components/git/deploymentSource.svelte` around lines 29 - 41, The
local state variable authorized is declared without an explicit type; update the
declaration of authorized to include an explicit type (matching other components
like multiSelectTable.svelte) such as boolean | null (or the specific
Writable/State type you use) so its intended shape is clear and consistent,
leaving the existing initializer ($state(null)) and keeping loadAuthorized and
its assignment (authorized = repository.authorized) unchanged.

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

src/lib/components/git/deploymentSource.svelte (1)

57-93: ⚠️ Potential issue | 🟠 Major

Calling loadRepository() in {#await} triggers duplicate API requests.

This issue was previously flagged: calling an async function directly inside {#await loadRepository()} creates a new Promise on every invalidation. When repository state updates on line 44, it triggers a re-render, which re-evaluates the await expression and makes another API call.

Use a $effect() to fetch once when dependencies change, and track loading state separately:

♻️ Suggested fix

 let repository = $state<Models.ProviderRepository | null>(null);
+let loading = $state(false);

-async function loadRepository() {
-    if (!resource?.installationId || !resource?.providerRepositoryId || !region || !project) {
-        return;
-    }
-
-    try {
-        repository = await sdk.forProject(region, project).vcs.getRepository({
-            installationId: resource.installationId,
-            providerRepositoryId: resource.providerRepositoryId
-        });
-    } catch (err) {
-        console.warn(err);
-    }
-}
+$effect(() => {
+    if (!resource?.installationId || !resource?.providerRepositoryId || !region || !project) {
+        return;
+    }
+
+    loading = true;
+    sdk.forProject(region, project)
+        .vcs.getRepository({
+            installationId: resource.installationId,
+            providerRepositoryId: resource.providerRepositoryId
+        })
+        .then((repo) => {
+            repository = repo;
+        })
+        .catch((err) => {
+            console.warn(err);
+        })
+        .finally(() => {
+            loading = false;
+        });
+});

Then in the template:

-{`#await` loadRepository()}
+{`#if` loading}
     <Layout.Stack direction="row" gap="xs" alignItems="center">
         <Skeleton variant="line" width={100} height={20} />
     </Layout.Stack>
-{:then}
+{:else}
     <Layout.Stack direction="row" gap="xs" alignItems="center">
         ...
     </Layout.Stack>
-{/await}
+{/if}

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/lib/components/git/deploymentSource.svelte` around lines 57 - 93, The
template is calling loadRepository() directly inside the {`#await`} block which
recreates a new Promise on every re-render (e.g., when repository changes) and
triggers duplicate API requests; instead, call loadRepository once from a
reactive/effect or onMount and store its result in a stable variable (e.g.,
repositoryPromise or set repository + loading flags) and then use that stable
variable in the template's {`#await` repositoryPromise} (or check
loading/repository for conditional rendering). Update the code to stop invoking
loadRepository() in the await expression, move the call into a $effect() or
onMount to re-run only when intended dependencies change, and ensure repository
and loading state are updated by the loadRepository handler so the UI uses those
values rather than re-creating the Promise.

src/lib/components/git/deploymentSource.svelte (1)

73-90: Consider using Tooltip instead of nested Popover for hover-based warning.

The nested Popover requires a click to show the warning. If the intent is to show the warning on hover (as the PR screenshot suggests), consider using a Tooltip component instead, which would provide a more discoverable UX for the authorization warning.

However, if click-to-reveal is the desired interaction, this implementation is fine.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/lib/components/git/deploymentSource.svelte` around lines 73 - 90, The
current implementation uses a click-activated Popover (Popover with let:toggle
and Button on:click={toggle}) to show the authorization warning, but the
reviewer suggests a hover-based Tooltip for better discoverability; replace the
Popover/ Button/ svelte:fragment tooltip block with the project's Tooltip
component (rendering the same Typography.Text and Link content) so the message
appears on hover, or if click behavior is intended, keep Popover but make that
explicit in the UI/props; locate the block using Popover, Button, toggle,
IconExclamation, and repository.providerInstallationId to apply the change.